Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed NetworkNeighbors #198

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jayantpranjal0
Copy link

Deprecate NetworkNeighbors

@jayantpranjal0 jayantpranjal0 marked this pull request as draft February 24, 2025 09:53
@matthyx
Copy link
Contributor

matthyx commented Feb 24, 2025

btw, when you create commits, can you use the -s option to add the Signed-off-by: footer to message?

@matthyx matthyx self-assigned this Feb 24, 2025
@jayantpranjal0
Copy link
Author

jayantpranjal0 commented Feb 24, 2025

Yeahh, I will make sure to do it. I actually wanted to resolve the codegen issue first, and then merge the two commits and then signoff.

Signed-off-by: JAYANT PRANJAL <[email protected]>
@jayantpranjal0 jayantpranjal0 force-pushed the deprecate_NetworkNeighbors branch from 031daed to 1d251cf Compare February 24, 2025 10:04
@matthyx
Copy link
Contributor

matthyx commented Feb 25, 2025

@jayantpranjal0 why did you remove networkpolicy/v1 and moved up the content of v2 ? This is the surest way of breaking API compabitility.

@jayantpranjal0
Copy link
Author

I found the difference between networkpolicy/v1 and networkpolicy/v2 to be that v1 implements the functions using NetworkNeighbors, whereas v2 does so through NetworkNeighborhood. There was one more minor change in a function mergeEgressRulesByPorts but both did the same thing, only a minor change in the way an object was created. So, ig v1 was for NetworkNeighbors compatibility. So, I removed it and moved up the contents of v2 to rather just use it as default, as we had to remove NetworkNeighbors completelely.

@matthyx
Copy link
Contributor

matthyx commented Feb 26, 2025

I found the difference between networkpolicy/v1 and networkpolicy/v2 to be that v1 implements the functions using NetworkNeighbors, whereas v2 does so through NetworkNeighborhood. There was one more minor change in a function mergeEgressRulesByPorts but both did the same thing, only a minor change in the way an object was created. So, ig v1 was for NetworkNeighbors compatibility. So, I removed it and moved up the contents of v2 to rather just use it as default, as we had to remove NetworkNeighbors completelely.

please don't do that... you can modify the v1 functions to panic but keep the existing paths

@jayantpranjal0
Copy link
Author

Okayy, I'll modify the v1 functions to panic instead of removing them completely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Waiting on Author
Development

Successfully merging this pull request may close these issues.

2 participants